Conversation
| assertType('*NEVER*', range(5, 6, 3)); | ||
| assertType('*NEVER*', range(2, 5, -1)); | ||
| assertType('*NEVER*', range('a', 'z', -1)); | ||
| assertType('*NEVER*', range(1, 10, INF)); |
There was a problem hiding this comment.
we need a test that range(1, 10, INF) will be false not *NEVER* on PHP7
There was a problem hiding this comment.
Added in 4f4f737. PHP 7 reported an invalid step with a warning and a false return value, so on PHP 7 the extension now returns false for such a call instead of an array type. RangePhp74Test runs with phpVersion: 70400 and covers range(1, 10, INF), range(2, 5, 0), range(5, 6, 3), and array{6}|false for a union where only one combination fails. PHP 7.4, 8.0–8.2 and 8.3+ all reject these steps, so the test runs on every CI runtime without a version gate.
5b68933 fixes the failing CI jobs: PHPUnit 11 treats an empty data provider as an error, so RangePhp82Test now skips through #[RequiresPhp('>= 8.3.0')].
|
This pull request has been marked as ready for review. |
| // PHP 7 reports an invalid step by returning false instead of throwing a ValueError | ||
| if ($rangeValues === false) { | ||
| if ($this->failsOnAnalysedVersion($startConstant->getValue(), $endConstant->getValue(), $stepConstant->getValue())) { | ||
| if ($this->phpVersion->throwsValueErrorForInternalFunctions()) { |
There was a problem hiding this comment.
if you use $scope->getPhpVersion() in the extension instead of the DI wide PhpVersion, type inference gets more precise and also will support narrowing via conditions like if (PHP_VERSION_ID < 80000) { .
this means you can put your tests into the nsrt/ folder and describe php-version dependent types like
if (PHP_VERSION_ID < 80000) {
// PHP 7 reports an invalid step with a warning and returns false instead of throwing a ValueError
assertType('false', range(1, 10, INF));
assertType('false', range(2, 5, 0));
...
}
without the need for adding a class RangePhp74Test extends TypeInferenceTestCase and the corresponding NEON will also be obsolete -> less files to fiddle with
There was a problem hiding this comment.
Done in 596668a. The extension reads $scope->getPhpVersion() now. I added throwsValueErrorForInternalFunctions() and hasStricterRangeFunction() to PhpVersions and dropped the PhpVersion method this PR had added. I moved the asserts to nsrt/range-invalid-step.php and nsrt/bug-10022.php, split by PHP_VERSION_ID, so RangePhp74Test, RangePhp82Test and the NEON file are gone.
With a version range, the runtime can reject a step that some analysed version accepts. The extension falls back to the general type in that case. Before, range($flag ? 2 : 5, 5, -1) analysed for 8.2 gave array{5} and missed [2, 3, 4, 5].
|
The Mutation Testing jobs fail on |
|
This pull request has been marked as ready for review. |
596668a to
3695000
Compare
|
After I opened this PR, d52db07 landed on 2.2.x: the extension no longer calls Current 2.2.x against this PR, with PHPStan running on PHP 8.5 (
Analysing for 8.2, |
845c934 to
4a20caf
Compare
PHP 8.0-8.2 ignore the sign of the step and, for numeric boundaries, reject one that is 0 or wider than the range; PHP 7 does the same with a warning and false instead of a ValueError. PHP 8.3 checks a step of 0, a NAN step and a step of PHP_INT_MIN before the boundaries, rejects a negative step on an increasing range, and builds a character range from two single bytes, in which a float step turns every character but a digit into 0. RangeFunctionArgumentsHelper::rejects() answers for the PHP versions of the scope, with PhpVersions::hasStricterRangeFunction() telling them apart. It uses the verdict of calling range() only when PHPStan itself runs on PHP 8.3 or newer, and leaves undecided what it cannot tell without running an older PHP: string boundaries before PHP 8.3, and numbers beyond 2 ** 53, which PHP compares exactly as integers. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
4a20caf to
9abe059
Compare
An invalid step makes range() throw a ValueError on PHP 8 and return
false with a warning on PHP 7, so a call where every combination of the
constant arguments is rejected is now `*NEVER*` on PHP 8 and `false` on
PHP 7. The extension folds the constant arguments by calling range() on
the PHP version PHPStan runs on, so RangeFunctionArgumentsHelper decides
whether the PHP versions of the scope reject the step as well. A
combination it cannot decide adds the general type to the union, next to
the types of the other combinations.
Long ranges are generalized from the values the range consists of, so
range('A', 'z') is a list of single byte strings and, since PHP 8.3,
where an integral float step stopped producing floats, range(1, 200, 1.0)
is a list of ints. Before PHP 8.3 a float argument still produces floats.
A range longer than ARRAY_COUNT_LIMIT, for which the extension skips
calling range(), gets the same rules from its argument types, including
the PHP 8.3 rejection of a negative step on an increasing range.
Ref phpstan/phpstan#10022
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
9abe059 to
fc6b43c
Compare
|
This pull request has been marked as ready for review. |
|
Dunno what you think @staabm ; but it's maybe simpler/better to target 2.3.x for all those new PR ? |
|
I rebased this PR and #6539 onto 2.3.x locally. The only conflict was in |
Ref phpstan/phpstan#10022
Two inaccuracies in the return type of
range():$stepmakesrange()throw aValueError, but the extension still returned an array type. It now returnsneverwhen every combination of the constant arguments throws. PHP 7 returnedfalsewith a warning instead, so on PHP 7 it returnsfalse. When the extension cannot decide a combination, the general type joins the types of the other combinations in the union.RANGE_LENGTH_THRESHOLDthe extension generalized the argument types, which predates the PHP 8.3 changes. It now generalizes the values the range consists of:// range('A', 'z') -non-empty-list<int|(literal-string&lowercase-string&non-falsy-string)|(literal-string&non-falsy-string&uppercase-string)> +non-empty-list<literal-string&non-empty-string> // range(1, 200, 1.0) -non-empty-list<float> +non-empty-list<int<1, 200>>Since d52db07 the extension skips calling
range()for a range longer thanARRAY_COUNT_LIMIT. On that path it applies the same rules to the argument types, and it rejects a negative$stepon an increasing range on PHP 8.3+.The extension folds the constant arguments by calling the native
range(), so a caughtValueErroronly says that the runtime rejects the step. I gated both changes onPhpVersion$scope->getPhpVersion()wherever PHP 8.3 changed the behaviour, so conditions likePHP_VERSION_ID < 80300narrow them. A$stepof0, or one wider than the range, has been aValueErrorsince PHP 8.0, unless both boundaries are equal and not both ints, while a negative$stepon an increasing range and a non-finite$steponly became one in 8.3. Before 8.3 an infinite$stepfailed as one wider than the range. An integral float$stepproduces ints since 8.3 and floats before that. Analysing for PHP 8.2 still givesnon-empty-list<int>forrange(2, 5, -1)andnon-empty-list<float>forrange(1, 200, 1.0).The tests inRangePhp82Testcovers that axis: it setsphpVersionto 8.2 and runs on 8.3+.nsrt/cover that axis withPHP_VERSION_IDconditions. The checks live inRangeFunctionArgumentsHelper, which this PR shares with #6539.This does not close phpstan/phpstan#10022. PHPStan already infers the PHP 8.3 result for the reproducer in that issue,
range('1', 'a'), because the extension folds the constant arguments through the nativerange(). For the same reason the folded values follow the PHP version PHPStan runs on rather than the configuredphpVersion: withphpVersion: 80200on a PHPStan running on 8.5,range('1', 'a')still comes out as the 49 element character range instead ofarray{1, 0}. Making the values followphpVersionmeans reimplementingrange()down to the rounding ofstart + i * step, and I did not want to put that in this PR. This PR fixes what you can decide without knowing the values: which steps PHP rejects, and how to generalize a range past the threshold.